feat!: Use artifactory catalog by yejseo01 · Pull Request #377 · arm/topo

yejseo01 · 2026-07-27T17:07:37Z

Changes

Loads the catalog and schema from Artifactory.
Removes the embedded catalog and builtin source.
Makes --source always available.
Adds shared HTTP fetching for catalog, install, upgrade, and version checks.
Fixes the --source registration panic.
Adds fetcher tests.

Checklist

🤖 This change is covered by tests as required.
🤹 All required manual testing has been performed.
📖 All documentation updates are complete.

muchzill4 · 2026-07-28T09:56:45Z

+		return nil, fmt.Errorf("sending request: %w", err)
+	}
+
+	if response.StatusCode < http.StatusOK || response.StatusCode >= http.StatusMultipleChoices {


Why not != http.StatusOK?

well, since we are trying to make it generic, I thought we might want to accept 204 No Content and 206 Partial Content, and the caller would handle such. Can stick to != http.StatusOK if you think that's better?

How would caller handle them right now?

right now, they just check != http.StatusOK. Maybe you're right, and we won't need it. Will fix it to != http.StatusOK

done - 09ac673

muchzill4 · 2026-07-28T10:04:49Z

Is fix a good category for this? It’s not fixing a bug.

awphi · 2026-07-28T16:01:03Z

+const (
+	DefaultURL        = "https://artifacts.tools.arm.com/devx-topo-project-catalog/" + version + "/catalog/"
+	DefaultCatalogURL = DefaultURL + "catalog.json"
+	DefaultSchemaURL  = DefaultURL + "catalog.schema.json"


It feels a little unfortunate to download the schema for artifactory every time for every type of source. It means even users who use file:// sources require a network connection + slows the command down to download a static file every time.

Maybe fine for a v1 but wonder if we should consider downloading + embedding the schema version that the CLI supports into the binary? Especially since the catalogDocument type is a mirror of the schema.

Bonus - wonder if we should generate go types based on the schema?!

Come to think of it, you’re right @awphi, this is a bit wasteful. I wonder if generating types based on the schema would be better. We could also do a pre-flight check of the $schema field in the received catalog - if it matches our expectation.

yea I was thinking this on the way back home. For Bartek's suggestion, I'm not sure how that would alleviate user with file:// source from needing network connection? I think to address that, we have no choice but to keep the schema in the same repository

Another thing we can try is to fetch the schema on release of topo, and embed that in the binary on release

yejseo01 · 2026-07-30T10:46:20Z

for error messages to precisely compare the versions, this PR needs to be merged first to continue - arm/topo-project-catalog#8

awphi

Nice! Have approved arm/topo-project-catalog#8 so we can add the major version check.

awphi · 2026-07-31T14:01:01Z

-
-	compiler := jsonschema.NewCompiler()
-	schemaDoc, err := jsonschema.UnmarshalJSON(bytes.NewReader(catalogSchemaJSON))
+func parseProjects(ctx context.Context, b []byte) ([]Project, error) {


ctx is unused

done - 4088273

awphi · 2026-07-31T14:04:50Z

+```sh
+go run ./scripts/generate_catalog_types v1
+```


Was //go generate not worth the trouble? I kind of like the idea of running all of our codebase dev tasks through standard go-supported paths if possible, not a blocker ofc

if you like it, no problem using it. I was thinking if someone is not familiar with go generate, it's one less thing for them to read about if we keep it this way though. Basically does the same thing, but with directives, it's more looking up in the internet than just reading the code in my perspective

On the flipside, it's nicer since you learn what go generate is once, then you can use it across any Go project rather than learning project-specific idioms like go run ./scripts/..., Makefiles, Justfiles or whatever people choose to use. My understanding is that's the whole point of go generate - standardisation.

awphi · 2026-07-31T14:05:29Z

+
+Topo uses the catalog version declared in `internal/catalog/catalog_schema_generated.go`. The same version selects both the catalog and its schema from Artifactory.
+
+Generating the Go types requires Node.js 20 or newer, `npx`, and access to Artifactory. Pass the catalog version to the generator:


Is it worth calling out access to artifactory? It's public so anyone with an internet connection should have access right?

yes. I'll get rid of this mention

done - 1b69da0

awphi · 2026-07-31T14:12:16Z

+const (
+	quicktypeVersion     = "26.0.0"
+	defaultSchemaBaseURL = "https://artifacts.tools.arm.com/devx-topo-project-catalog/"
+	generationTimeout    = 2 * time.Minute


Is a timeout strictly necessary? Think we can get away without it for an internal dev tool like this that does a pretty simple task

good catch. I agree

done - e6921e1

awphi · 2026-07-31T14:18:36Z

+	formatted, err := addCatalogVersionConstant(generated, catalogVersion)
+	if err != nil {
+		return fmt.Errorf("failed to format quicktype output from schema %q: %w", schemaURL, err)
+	}
+	// #nosec G703 -- outputFile is derived from the generator source location.
+	if err := os.WriteFile(outputFile, formatted, 0o644); err != nil {
+		return fmt.Errorf("failed to write generated types to %q: %w", outputFile, err)
+	}


Is building an AST worth it here? Couldn't we just inject the const at the end of the file? e.g.

Suggested change

formatted, err := addCatalogVersionConstant(generated, catalogVersion)

if err != nil {

return fmt.Errorf("failed to format quicktype output from schema %q: %w", schemaURL, err)

}

// #nosec G703 -- outputFile is derived from the generator source location.

if err := os.WriteFile(outputFile, formatted, 0o644); err != nil {

return fmt.Errorf("failed to write generated types to %q: %w", outputFile, err)

}

formatted := string(generated) + "\n" + "const CatalogMajorVersion = \"" + majorVersion(catalogVersion) + "\"\n"

// #nosec G703 -- outputFile is derived from the generator source location.

if err := os.WriteFile(outputFile, []byte(formatted), 0o644); err != nil {

return fmt.Errorf("failed to write generated types to %q: %w", outputFile, err)

}

I guess we can? it's just quite awkward to have const at the bottom in my instinct

yea, I think we should judging by how much code we save

done - ce1b9fa

awphi · 2026-07-31T14:22:11Z

+func majorVersion(version string) string {
+	major, _, _ := strings.Cut(version, ".")
+	return major
+}


It might be useful to store the full version in the const for us as maintainers if nothing else so we know what version of the schema a given version of the CLI uses without manually diffing our go types vs the source json schema.

I agree. making this change now. Also we don't have the messge comparing the schema and the catalog version yet. making this as well

done - b832259

awphi · 2026-07-31T14:26:48Z

+// Advisory metadata that implementations may use to discover, filter, or suggest suitable
+// parameter values. Unknown hint keys should be ignored.
+type Hints struct {
+}


Why has this been generated empty? Does quicktype not support patternProperties?

oh good catch. It seems like patternProperties are not supported in quicktype. Does Hints need to be patternProperties?

yejseo01 requested a review from a team as a code owner July 27, 2026 17:07

muchzill4 requested changes Jul 28, 2026

View reviewed changes

yejseo01 changed the title ~~fix!: Use artifactory catalog~~ feat!: Use artifactory catalog Jul 28, 2026

awphi reviewed Jul 28, 2026

View reviewed changes

yejseo01 added 8 commits July 29, 2026 14:58

refactor: share HTTP fetch logic

dac3d33

feat: load project catalog from Artifactory

b1fe262

fix: always register projects source flag

9f947a2

remove unnecessary comment

ee0cf35

style: format catalog constants

a25db80

refactor: use catalog URL as source default

2040321

refactor: let callers control fetch timeout

051f6ad

refactor: generate catalog types from schema

5b3f922

yejseo01 force-pushed the use-artifactory-catalog branch from ed774fc to 5b3f922 Compare July 29, 2026 13:59

feat: derive catalog major version from schema release

d948c4f

yejseo01 added 2 commits July 31, 2026 11:56

simplify error condition for url fetching

09ac673

test: require OK status for fetch responses

4bbfd1d

awphi reviewed Jul 31, 2026

View reviewed changes

yejseo01 added 5 commits July 31, 2026 16:15

refactor: remove unused catalog parsing context

4088273

docs: clarify catalog generator network requirement

1b69da0

refactor: remove catalog generation timeout

e6921e1

refactor: append catalog version to generated types

ce1b9fa

feat: report incompatible catalog schema versions

b832259


		Topo uses the catalog version declared in `internal/catalog/catalog_schema_generated.go`. The same version selects both the catalog and its schema from Artifactory.

		Generating the Go types requires Node.js 20 or newer, `npx`, and access to Artifactory. Pass the catalog version to the generator:

Uh oh!

Conversation

yejseo01 commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Checklist

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

muchzill4 commented Jul 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yejseo01 Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yejseo01 commented Jul 30, 2026

Uh oh!

awphi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yejseo01 commented Jul 27, 2026 •

edited

Loading

yejseo01 Jul 29, 2026 •

edited

Loading

yejseo01 Jul 31, 2026 •

edited

Loading